Skip to content

add withRemove() and tests#936

Closed
digital88 wants to merge 1 commit intotestcontainers:mainfrom
digital88:withReuse
Closed

add withRemove() and tests#936
digital88 wants to merge 1 commit intotestcontainers:mainfrom
digital88:withReuse

Conversation

@digital88
Copy link
Copy Markdown
Contributor

@digital88 digital88 commented Mar 17, 2025 •
edited
Loading

Related issue #850

Add withRemove() method to GenericContainer and tests. This allows to control autoremoval behaviour of a container.

Containers are removed by default if remove option is not set when container is being stopped.

await container.stop(); // will remove stopped container.

withRemove(true) produces the same effect as explicitly setting remove flag when stopping container.

container.withRemove(true).start();
...
await container.stop();

// is equivalent of doing:

await container.stop({ remove: true });

// is equivalent of doing:

await container.stop();

withRemove(false) disables removing container by default.

container.withRemove(false).start();
...
await container.stop(); // will NOT remove stopped container.

container.stop() options have higher priority than withRemove()

container.withRemove(false).start();
...
await container.stop({ remove: true }); // will override withRemove(false) and remove stopped container.

Keep in mind that Ryuk will remove stopped container after tests finished regardless of value passed in withRemove method, unless Ryuk is disabled. Setting withRemove(false) and disabling Ryuk is not recommeded.

@netlify
Copy link
Copy Markdown

netlify bot commented Mar 17, 2025 •
edited
Loading

✅ Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit 6465ab0
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/67d8954f9d245500077e14d6
😎 Deploy Preview https://deploy-preview-936--testcontainers-node.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@cristianrgreco cristianrgreco added enhancement New feature or request minor Backward compatible functionality labels Mar 17, 2025
@cristianrgreco
Copy link
Copy Markdown
Collaborator

cristianrgreco commented Mar 18, 2025 •
edited
Loading

Hi @digital88, thanks for raising the PR, but it is very similar to #905. The other PR is a bit further progressed, e.g it updates the docs, and the testcontainer interface. It looks like both PRs are now failing for the same reason, some difference with the container name.

Instead of asserting the container name does or does not exist, is it not possible to assert the container IDs? That should resolve any inconsistency with whether the name is prefixed with / or not.

Would it be possible for you to collaborate on PR #905 instead of creating a new one?

@digital88
Copy link
Copy Markdown
Contributor Author

digital88 commented Mar 18, 2025

Would it be possible for you to collaborate on PR #905 instead of creating a new one?

Yes, I'll look into it.

Instead of asserting the container name does or does not exist, is it not possible to assert the container IDs?

Yes, I was thinking about using container id too. Will try this idea in PR #905

@digital88
Copy link
Copy Markdown
Contributor Author

digital88 commented Mar 18, 2025

See #939

@digital88 digital88 closed this Mar 18, 2025
@digital88 digital88 deleted the withReuse branch March 19, 2025 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement New feature or request minor Backward compatible functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@digital88 @cristianrgreco